Skip to content

Conversation

@fhenneke
Copy link
Contributor

@fhenneke fhenneke commented Nov 13, 2025

This PR adds an explicit check for order type instead of relying on checking tokens.

This should prevent regressions if we enable the case of orders with the sell token being equal to the buy token.

Rational

The code change is only affecting a part of the classified fees query which was ensuring backwards compatibility for missing network fee data. Then network fees had to be reconstructed differently for buy and sell orders. That part of the code will probably never be used for new orders with buy and sell token being equal. The changed code is, however, more explicit and prevents a regression if we should ever rely on that part of the code for new orders.

Testing

  • A test query using the new code can be found here.
  • A query comparing results of the new and old query can be found here. There are no differences in the output comparing one week of data on ethereum (90k entries). I also checked the week 1 year before and results look good.

Since this change does not affect current data and produces the exact same results for two test weeks, merging it will be low risk.

@fhenneke fhenneke marked this pull request as ready for review November 14, 2025 09:46
@fhenneke fhenneke changed the title Check explicitly for order type Check explicitly for order type in classified fees Nov 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants